Repository navigation
feat(core): Enrich MCP SDK spans instead of duplicating them - #26309
Draft
sentry-junior[bot] wants to merge 6 commits into
Draft
sentry-junior[bot] wants to merge 6 commits into
sentry-junior[bot] wants to merge 6 commits into
Conversation
@modelcontextprotocol/server starts its own OpenTelemetry span for every MCP request and notification once a tracer provider is registered. With the Sentry tracer provider (e.g. @sentry/nextjs, @sentry/sveltekit, @sentry/deno, or enableOpenTelemetrySetup: true) every request ended up with two server spans. The MCP server instrumentation now adds its attributes to the span the MCP SDK starts and only starts its own span when the SDK records nothing. Refs #26078 Co-Authored-By: Francesco Gringl-Novy <francesco.novy@sentry.io>
Contributor
size-limit report 📦Compared: 302a18b (PR test merge using develop at 27bc7cb) Against: 27bc7cb (develop) · base run #65789 ✅ Baseline matches: this diff contains only this PR's changes.
|
Contributor
|
One or more gzipped bundles increased by more than 500 bytes. If this increase is intentional, add the Accept Bundlesize Increase label to this PR to rerun and accept the check. |
Contributor
Optional E2E testsSome optional checks were not successful. They do not block the merge. 5 not successful, 56 successful. See the run.
Successful checks
|
mydea
reviewed
Nov 12, 2026
mydea
reviewed
Nov 12, 2026
mydea
reviewed
Nov 12, 2026
mydea
reviewed
Nov 12, 2026
Co-Authored-By: Francesco Gringl-Novy <francesco.novy@sentry.io>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why: The next
@modelcontextprotocol/serverrelease starts its own OpenTelemetry span for every MCP request and notification whenever a global tracer provider is registered (modelcontextprotocol/typescript-sdk#3115). With the Sentry tracer provider registered, every request gets two nested server spans with the same name. That happens by default in nextjs, sveltekit, deno and vercel-edge, and anywhereenableOpenTelemetrySetup: trueis set. This change adds our attributes to the span the MCP SDK starts. We only start our own span when the SDK records nothing. Upstream's docs tell Sentry users to remove theMcpServerintegration instead, but that loses tool arguments and results, which upstream deliberately doesn't record.Non-obvious decisions:
Protocol._instrumentation. That field is package-internal, but the integration already reads_registeredToolsand similar. Without the field (v1, v2 ≤ 2.4,openTelemetry: false), nothing changes.@sentry/nodedefault), the existing code path runs unchanged. If a non-Sentry provider is registered, the SDK's span never reaches Sentry, so we start our own span inside the SDK's scope.transport.send. Response attributes (tool/prompt results,initializeserver info) are therefore applied from the SDK's end callback, not from send-time correlation.How to review: Start with
upstreamInstrumentation.ts, then the hand-off branches intransport.ts. The changes inspans.tsandcorrelation.tsonly separate building attributes from starting or ending the span, so both paths can use them.Unsure about:
http.serverspan instead of its own segment, becauseforceTransactioncan't be applied after the span is started. Is that fine for the MCP insights views?node-express-mcp-v2 (MCP SDK OpenTelemetry)installs the unreleased SDK from pkg.pr.new, pinned to typescript-sdk919bb8e. It disablesblock-exotic-subdepsbecause the previews depend on each other by URL. Once upstream releases, it should switch to npm.Refs #26078
Verification
upstreamInstrumentation.test.tscovers hand-off for requests,initialize, and incoming/outgoing notifications. It also covers the fallback when a non-Sentry provider is registered, when no provider is registered, and when the SDK has no instrumentation. The four hand-off tests fail without the fix.node-express-mcp-v2 (MCP SDK OpenTelemetry)passes locally. With only the core change reverted, it fails with twoinitializespans.node-express-mcp-v2andnode-express-mcp-v1, the node-integration-testsmcp-server,mcp-server-oauth-clientandmcp-handler-exact-once(30 tests), and the coremcp-server+tracingunit tests.via Francesco Gringl-Novy.
--
View Session [Sentry]